REF/TST: Add more pytest idiom to mi indexing tests #24040

simonjayhawkins · 2018-12-01T23:54:19Z

[n/a ] follow on from TST: split up pandas/tests/indexing/test_multiindex.py #23912
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
[n/a ] whatsnew entry

for this pass:

the Test classes have been removed.
catch_warnings used for the .ix deprecation replaced with @pytest.mark.filterwarnings("ignore:\\n.ix:DeprecationWarning") at test level.

pep8speaks · 2018-12-01T23:54:26Z

Hello @simonjayhawkins! Thanks for submitting the PR.

There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_getitem.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_iloc.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_ix.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_loc.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_multiindex.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_panel.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_partial.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_set_ops.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_setitem.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_slice.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_sorted.py !
There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_xs.py !

codecov · 2018-12-02T00:29:16Z

Codecov Report

Merging #24040 into master will increase coverage by 49.88%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #24040       +/-   ##
===========================================
+ Coverage   42.46%   92.35%   +49.88%     
===========================================
  Files         161      161               
  Lines       51557    51557               
===========================================
+ Hits        21892    47613    +25721     
+ Misses      29665     3944    -25721

Flag	Coverage Δ
#multiple	`90.75% <ø> (?)`
#single	`42.46% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/computation/pytables.py	`92.37% <0%> (+0.3%)`	⬆️
pandas/io/pytables.py	`92.3% <0%> (+0.92%)`	⬆️
pandas/util/_test_decorators.py	`93.24% <0%> (+4.05%)`	⬆️
pandas/compat/__init__.py	`58.36% <0%> (+8.17%)`	⬆️
pandas/core/config_init.py	`99.24% <0%> (+9.84%)`	⬆️
pandas/core/reshape/util.py	`100% <0%> (+11.53%)`	⬆️
pandas/compat/numpy/__init__.py	`92.85% <0%> (+14.28%)`	⬆️
pandas/core/computation/common.py	`85.71% <0%> (+14.28%)`	⬆️
pandas/core/api.py	`100% <0%> (+14.81%)`	⬆️
pandas/core/indexes/api.py	`99% <0%> (+14.85%)`	⬆️
... and 119 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b45eb26...c97de72. Read the comment docs.

codecov · 2018-12-02T00:29:17Z

Codecov Report

Merging #24040 into master will increase coverage by 49.88%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #24040       +/-   ##
===========================================
+ Coverage   42.46%   92.35%   +49.88%     
===========================================
  Files         161      161               
  Lines       51557    51557               
===========================================
+ Hits        21892    47613    +25721     
+ Misses      29665     3944    -25721

Flag	Coverage Δ
#multiple	`90.75% <ø> (?)`
#single	`42.46% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/computation/pytables.py	`92.37% <0%> (+0.3%)`	⬆️
pandas/io/pytables.py	`92.3% <0%> (+0.92%)`	⬆️
pandas/util/_test_decorators.py	`93.24% <0%> (+4.05%)`	⬆️
pandas/compat/__init__.py	`58.36% <0%> (+8.17%)`	⬆️
pandas/core/config_init.py	`99.24% <0%> (+9.84%)`	⬆️
pandas/core/reshape/util.py	`100% <0%> (+11.53%)`	⬆️
pandas/compat/numpy/__init__.py	`92.85% <0%> (+14.28%)`	⬆️
pandas/core/computation/common.py	`85.71% <0%> (+14.28%)`	⬆️
pandas/core/api.py	`100% <0%> (+14.81%)`	⬆️
pandas/core/indexes/api.py	`99% <0%> (+14.85%)`	⬆️
... and 119 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b45eb26...c97de72. Read the comment docs.

gfyoung

This is awesome work! However, I have to ask:

Would it be possible to split this PR into multiple PR's? This would make things easier to review.

Also, if you spot opportunities to improve the tests when refactoring, those would be great as well! Improvements include:

Breaking up big tests into smaller ones
Checking error messages in pytest.raises if meaningful

simonjayhawkins · 2018-12-02T11:40:47Z

@gfyoung thanks for the review. breaking down the PR is not a problem. would your preference be to split by activity, by file or perhaps limit the number of files in the PR?

jreback · 2018-12-02T17:53:47Z

pandas/tests/indexing/multiindex/test_iloc.py

 import numpy as np
 import pytest

 from pandas import DataFrame, MultiIndex, Series
 from pandas.util import testing as tm


-@pytest.mark.filterwarnings("ignore:\\n.ix:DeprecationWarning")
-class TestMultiIndexIloc(object):
+def test_iloc_getitem_multiindex2():


we should fix this :>

xref #24272

jreback · 2018-12-02T17:55:11Z

@simonjayhawkins here i think you can split by files. e.g. try to limit the amount that requires review in any 1 PR. Also if you have a PR which simply moves things, those should be independent to making actual changes.

gfyoung · 2018-12-02T19:05:05Z

here i think you can split by files.

I second @jreback on this.

remove test classes and replace catch_warnings

c97de72

gfyoung added Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite MultiIndex labels Dec 2, 2018

gfyoung requested changes Dec 2, 2018

View reviewed changes

jreback reviewed Dec 2, 2018

View reviewed changes

simonjayhawkins closed this Dec 2, 2018

gfyoung added this to the No action milestone Dec 2, 2018

simonjayhawkins mentioned this pull request Dec 2, 2018

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24053

Merged

This was referenced Dec 13, 2018

REF/TST: Add more pytest idiom to indexing/multiindex/test_iloc.py #24272

Merged

REF/TST: Add more pytest idiom to indexing/multiindex/test_xs.py #24370

Merged

simonjayhawkins deleted the mi-test-clean branch December 24, 2018 09:20

This was referenced Dec 24, 2018

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24409

Merged

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24452

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REF/TST: Add more pytest idiom to mi indexing tests #24040

REF/TST: Add more pytest idiom to mi indexing tests #24040

simonjayhawkins commented Dec 1, 2018 •

edited

Loading

pep8speaks commented Dec 1, 2018

codecov bot commented Dec 2, 2018

codecov bot commented Dec 2, 2018 •

edited

Loading

gfyoung left a comment

simonjayhawkins commented Dec 2, 2018

jreback Dec 2, 2018

simonjayhawkins Dec 13, 2018

jreback commented Dec 2, 2018

gfyoung commented Dec 2, 2018

REF/TST: Add more pytest idiom to mi indexing tests #24040

REF/TST: Add more pytest idiom to mi indexing tests #24040

Conversation

simonjayhawkins commented Dec 1, 2018 • edited Loading

pep8speaks commented Dec 1, 2018

codecov bot commented Dec 2, 2018

Codecov Report

codecov bot commented Dec 2, 2018 • edited Loading

Codecov Report

gfyoung left a comment

Choose a reason for hiding this comment

simonjayhawkins commented Dec 2, 2018

jreback Dec 2, 2018

Choose a reason for hiding this comment

simonjayhawkins Dec 13, 2018

Choose a reason for hiding this comment

jreback commented Dec 2, 2018

gfyoung commented Dec 2, 2018

simonjayhawkins commented Dec 1, 2018 •

edited

Loading

codecov bot commented Dec 2, 2018 •

edited

Loading